Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Refresh tree shortcut #123

Merged
merged 6 commits into from
Nov 30, 2024
Merged

Conversation

gytic
Copy link
Contributor

@gytic gytic commented Nov 16, 2024

This PR should fix #118

To refresh the tree view my idea is to:

  1. clear existing nodes (under root)
  2. Re-initialize nodes (under root)
  3. Reset search state
  4. Close opened table if it no longer exists

@gytic
Copy link
Contributor Author

gytic commented Nov 16, 2024

@jorgerojas26 do you have an idea where and how to close the opened table if it was deleted?

I will look tomorrow a bit more into the code base to figure out how it is opened etc but maybe you have an idea already how one could implement that.

@jorgerojas26
Copy link
Owner

I'm not sure what you mean but maybe you just delete the tab that has the table open.

- refactored tree node init
- new function for reset
  - reset filter
  - delete old nodes -> init new nodes
  - TODO: close table view if table was deleted
@gytic
Copy link
Contributor Author

gytic commented Nov 17, 2024

Okay I think this is the best I can do.

When refreshing, all nodes under the root node get deleted and afterwards re added.

@gytic gytic changed the title WIP feat: Refresh tree shortcut feat: Refresh tree shortcut Nov 17, 2024
@gytic gytic marked this pull request as ready for review November 17, 2024 15:48
components/Tree.go Outdated Show resolved Hide resolved
@gytic
Copy link
Contributor Author

gytic commented Nov 25, 2024

PR is ready for review

@jorgerojas26
Copy link
Owner

This is good enough for now, i think. Can you please just change the keybinding from 'r' to 'R'. The reason is, there is a 'R' keymap for the table that refreshes the data, so, to keep consistency let's use 'R' also for the tree.

With that change, i'm ready to merge.

Thank you very much

@jorgerojas26
Copy link
Owner

Also, please check and fix the linter issues.

@gytic
Copy link
Contributor Author

gytic commented Nov 27, 2024

I will work on it this evening

- switched from 'r' to 'R' to be more consistent with other refresh shortcuts
@gytic
Copy link
Contributor Author

gytic commented Nov 27, 2024

I ran the lints locally and had no errors. I think this should be now fixed.

Also changed the shortcut from r to R.
PR should be ready for merge now.

@jorgerojas26 jorgerojas26 merged commit 78be965 into jorgerojas26:main Nov 30, 2024
2 checks passed
@gytic gytic deleted the refresh-tree branch December 1, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a table does not reflect in the TUI
3 participants